Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: cdk603 error calculating previousLocalExitRoot #199

Merged
merged 12 commits into from
Nov 27, 2024

Conversation

joanestebanr
Copy link
Contributor

@joanestebanr joanestebanr commented Nov 25, 2024

Description

Since agglayer:0.2.0-rc.15 the responses of CertificateHeader add a new field: PreviousLocalExitRoot

  • Fixes error calculating previousLocalExitRoot on new certificate if a certificate is inError

NOTE: Database incompatibility with previous version!, require to delete it!

@joanestebanr joanestebanr force-pushed the fix/cdk603-previousLER_error-v2 branch from 96c7e2d to 1d4224c Compare November 26, 2024 11:47
@joanestebanr joanestebanr marked this pull request as ready for review November 26, 2024 11:47
@joanestebanr joanestebanr changed the title Fix: cdk603 error calculating previousLocalExitRoot fix: cdk603 error calculating previousLocalExitRoot Nov 26, 2024
@joanestebanr joanestebanr marked this pull request as draft November 26, 2024 11:52
@joanestebanr joanestebanr marked this pull request as ready for review November 27, 2024 08:53
@joanestebanr joanestebanr merged commit 521b968 into release/v0.5.0 Nov 27, 2024
18 checks passed
@joanestebanr joanestebanr deleted the fix/cdk603-previousLER_error-v2 branch November 27, 2024 09:14
@@ -6,6 +6,7 @@ CREATE TABLE certificate_info (
height INTEGER NOT NULL,
certificate_id VARCHAR NOT NULL PRIMARY KEY,
status INTEGER NOT NULL,
previous_local_exit_root VARCHAR ,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
previous_local_exit_root VARCHAR ,
previous_local_exit_root VARCHAR,


return fmt.Sprintf("Height: %d, CertificateID: %s, NewLocalExitRoot: %s. Status: %s. Errors: [%s]",
c.Height, c.CertificateID.String(), c.NewLocalExitRoot.String(), c.Status.String(), errors)
previousLocalExitRoot := "nil"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

use the nilStr constant?

@@ -23,6 +23,8 @@ const (
Candidate
InError
Settled

nilStr = "nil"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this needed, agglayer API?

@@ -283,7 +285,7 @@ func TestAggSenderStart(t *testing.T) {
ctx,
log.WithFields("test", "unittest"),
Config{
StoragePath: "file::memory:?cache=shared",
StoragePath: "file:TestAggSenderStart?mode=memory&cache=shared",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are we 100% sure this is using memory or a file with this name?

Stefan-Ethernal pushed a commit that referenced this pull request Nov 27, 2024
Support `agglayer:0.2.0-rc.15` the responses of ` CertificateHeader` add a new field: `PreviousLocalExitRoot`
- Fixes error calculating  previousLocalExitRoot on new certificate if a certificate is inError

NOTE: **Database incompatibility with previous version!, require to delete it!**
Vui-Chee added a commit to okx/xlayer-cdk that referenced this pull request Dec 3, 2024
* tag 'v0.5.0-beta6':
  feat: write on database the number of retries per certificate and the certificates in a history table (0xPolygon#208)
  feat: return an error in case agglayer returns certificate with height lower than in local storage (0xPolygon#209)
  chore: simplify the `HashMeddler` (0xPolygon#205)
  fix: clean proof table on start (0xPolygon#207) (0xPolygon#210)
  feat: improve logs (0xPolygon#204)
  fix: cdk603 error calculating previousLocalExitRoot (0xPolygon#199)
  fix: Integration Bali PP (0xPolygon#198)
  feat: check agglayer certificate and use as initial if db is empty (0xPolygon#192)
  feat: sqlite aggregator (0xPolygon#189)
  feat: BridgeMessage e2e test (0xPolygon#184)
  feat: aggsender e2e (0xPolygon#183)
  fix: aggregating proofs (0xPolygon#191) (0xPolygon#193)
  feat: l1infotreesync can be run as individual component (0xPolygon#188)
  fix: l1infotree flaky test (0xPolygon#182)
  fix: `L1InfoRootIncorrect` error from `agglayer` (0xPolygon#185)
  feat: improve aggsender logs (0xPolygon#186) (0xPolygon#187)
  feat: remove sanity check (0xPolygon#178) (0xPolygon#179)
  refact: GetSequence method (0xPolygon#169)
  feat: epoch notifier (0xPolygon#144)
  feat: unpack and log agglayer errors (0xPolygon#158)
Vui-Chee added a commit to okx/xlayer-cdk that referenced this pull request Dec 4, 2024
* dev: (22 commits)
  fix issues
  feat: healthcheck (#11)
  feat: write on database the number of retries per certificate and the certificates in a history table (0xPolygon#208)
  feat: return an error in case agglayer returns certificate with height lower than in local storage (0xPolygon#209)
  chore: simplify the `HashMeddler` (0xPolygon#205)
  fix: clean proof table on start (0xPolygon#207) (0xPolygon#210)
  feat: improve logs (0xPolygon#204)
  fix: cdk603 error calculating previousLocalExitRoot (0xPolygon#199)
  fix: Integration Bali PP (0xPolygon#198)
  feat: check agglayer certificate and use as initial if db is empty (0xPolygon#192)
  feat: sqlite aggregator (0xPolygon#189)
  feat: BridgeMessage e2e test (0xPolygon#184)
  feat: aggsender e2e (0xPolygon#183)
  fix: aggregating proofs (0xPolygon#191) (0xPolygon#193)
  feat: l1infotreesync can be run as individual component (0xPolygon#188)
  fix: l1infotree flaky test (0xPolygon#182)
  fix: `L1InfoRootIncorrect` error from `agglayer` (0xPolygon#185)
  feat: improve aggsender logs (0xPolygon#186) (0xPolygon#187)
  feat: remove sanity check (0xPolygon#178) (0xPolygon#179)
  refact: GetSequence method (0xPolygon#169)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants